Repository navigation
feat(experiments): add retention metrics to the metric breakdown injector - #62292
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
This comment was marked as outdated.
This comment was marked as outdated.
b2daac5 to
819b8db
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
819b8db to
cfb273c
Compare
cfb273c to
e0c91db
Compare
e0c91db to
2df015d
Compare
2df015d to
ebfae47
Compare
29d5be1 to
3eb64ae
Compare
3eb64ae to
e09f392
Compare
e09f392 to
20f5ab8
Compare
20f5ab8 to
3d9888b
Compare
3d9888b to
095a579
Compare
095a579 to
ced8686
Compare
andehen
left a comment
There was a problem hiding this comment.
Findings from a review of the full stack (#61788, #61796, #62291, #62292, #62293), placed on the PR where each belongs. Prefixes: should-fix, suggestion, nit. Only one small thing here, the retention path is the most solid of the four (breakdown read off the start event keeps the per-bucket denominator well defined, and the non-retained-users test covers it).
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the |
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the |
ced8686 to
59c3368
Compare
59c3368 to
ef22067
Compare
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the |

Problem
Funnel (#61796) and mean (#62291) metrics attribute breakdowns from the metric event. Retention metrics still use the exposure-event injector.
Changes
This PR migrates retention metrics to
MetricBreakdownInjector, behind the same flag.Retention support in the injector
The breakdown is read off the start event and attributed first touch (
argMinover the start event timestamp). Attribution is fixed with no user choice, and that's deliberate: retention isretained / cohort, the cohort (denominator) is defined by the start event, and every retained user has one. Reading the breakdown off the completion event would break the per-bucket denominator, since non-retained users have no completion event to bucket by.start_eventsalready groups by entity, so the value is deduped per user there and carried intoentity_metrics.products/experiments/backend/hogql_queries/metric_breakdown_injector.pyproducts/experiments/backend/hogql_queries/experiment_query_runner.pyproducts/experiments/backend/hogql_queries/experiment_retention_query_builder.pyTests
Flag-on retention cases in
test_metric_breakdown.py(start-event attribution, limit, Other bucket) plus injector unit tests.products/experiments/backend/hogql_queries/test/experiment_query_runner/test_metric_breakdown.pyproducts/experiments/backend/hogql_queries/test/test_metric_breakdown_injector.pyHow did you test this code?
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Automatic notifications
Docs update
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Model: Opus 4.7 (implementation), Fable 5 (restack and PR description)
Manually refactored: yes
Skills used:
Relevant decisions: